Skip to content

Comments

Use global Stack as fallback#1867

Merged
lionello merged 2 commits intomainfrom
lio/fallback-stack
Jan 25, 2026
Merged

Use global Stack as fallback#1867
lionello merged 2 commits intomainfrom
lio/fallback-stack

Conversation

@lionello
Copy link
Member

@lionello lionello commented Jan 25, 2026

Description

Since we removed all occurrences of global.Stack with session.Stack, we ignore the flags (apart from --provider), resulting in accidentally downgrading Portal from HA to affordable.

This PR uses the global.Stack as the fallback stack.

Linked Issues

Checklist

  • I have performed a self-review of my code
  • I have added appropriate tests
  • I have updated the Defang CLI docs and/or README to reflect my changes, if necessary

Summary by CodeRabbit

  • Bug Fixes

    • Improved error message for invalid color inputs — shows the invalid value and available options.
  • Refactor

    • Consolidated stack and provider configuration into a unified default-based structure to streamline selection.
    • Updated stack resolution and fallback behavior so defaults and interactive selection provide more consistent fallback stacks.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 25, 2026

📝 Walkthrough

Walkthrough

Consolidates stack/provider inputs into a Default stacks.Parameters field on GetStackOpts, updates session loader and stacks.Manager to use that Default, removes a nil-stack-manager fallback, adjusts tests, and tweaks one CLI error message for color parsing. (48 words)

Changes

Cohort / File(s) Summary
Error messaging
src/cmd/cli/command/color.go
Clarified invalid color error text: changed from "color mode not one of %v" to invalid color: %q, not one of %v (quotes the invalid input).
CLI session opts
src/cmd/cli/command/session.go
Stop reading separate stack/provider flags into GetStackOpts; now populate GetStackOpts.Default with global.Stack.
Session loader
src/pkg/session/session.go
Removed early nil-stack-manager fallback in loadStack; always calls sl.sm.GetStack(ctx, sl.opts.GetStackOpts) and relies on stacks.Manager behavior.
Stacks manager & options
src/pkg/stacks/manager.go
GetStackOpts refactor: replaced ProviderID, Stack, AllowStackCreation with Default stacks.Parameters and embedded SelectStackOptions; GetStack now uses Default.Name / Default.Provider and returns Default-based fallbacks.
Tests
src/pkg/session/session_test.go, src/pkg/stacks/manager_test.go
Tests updated to use GetStackOpts.Default (Parameters) and new SelectStackOptions paths; removed TestLoadSession_NoStackManager; adjusted expectations for merged stack fields (Region, Variables).

Sequence Diagram(s)

sequenceDiagram
  participant CLI as Client CLI
  participant SL as SessionLoader
  participant SM as StacksManager
  participant DS as DataSource (local/remote)

  CLI->>SL: Build GetStackOpts (Default := global.Stack)
  SL->>SM: GetStack(ctx, GetStackOpts)
  alt Default specified
    SM->>SM: resolve getSpecifiedStack(Default.Name)
    SM->>DS: fetch stack data (local/remote)
    DS-->>SM: stack parameters
    SM-->>SL: Parameters (merged)
  else Interactive
    SM->>SM: getStackInteractively(opts.SelectStackOptions)
    SM-->>SL: Selected Parameters
  end
  SL-->>CLI: return resolved stack Parameters
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • jordanstephens

Poem

🐇 I hopped through options, Default in my paw,

Stacks and providers now settled in one law.
No more scattered flags or fallback hazy night—
Sessions call the manager, and everything's airtight.
— a rabbit, nibbling code with delight 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Use global Stack as fallback' directly and accurately summarizes the main objective of the pull request: restoring global.Stack as the fallback stack when session.Stack is not available.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.5.0)

level=warning msg="[linters_context] running gomodguard failed: unable to read module file go.mod: current working directory must have a go.mod file: if you are not using go modules it is suggested to disable this linter"
level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies"


Comment @coderabbitai help to get the list of available commands and usage tips.

@lionello lionello changed the title chore: same error for color as others Use global Stack as fallback Jan 25, 2026
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@src/pkg/session/session.go`:
- Around line 70-75: When sl.sm is nil in SessionLoader.loadStack, the code
currently unconditionally overwrites sl.opts.Fallback.Name with
stacks.DefaultBeta; change this so you only set fallback.Name =
stacks.DefaultBeta when the existing fallback.Name is empty. Locate
SessionLoader.loadStack and the fallback variable derived from sl.opts.Fallback,
and add a conditional check (e.g., if fallback.Name == "" or equivalent) before
assigning stacks.DefaultBeta, so a user-specified stack name is preserved when
the manager can't be created.
🧹 Nitpick comments (1)
src/pkg/stacks/manager.go (1)

286-291: Track the TODO for selector prefill.

The TODO indicates missing UX behavior (prefill from fallback). Please convert to a tracked issue or implement when feasible.

Copy link
Contributor

@edwardrf edwardrf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't quite like the name fallback, but I don't have a better idea.

use default options from command line, if set
@lionello lionello merged commit ff28e24 into main Jan 25, 2026
14 checks passed
@lionello lionello deleted the lio/fallback-stack branch January 25, 2026 16:43
@lionello lionello linked an issue Jan 25, 2026 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Regression: --mode flag ignored

2 participants